[CS2113-T14-1] SmartHomeBot#40
[CS2113-T14-1] SmartHomeBot#40zongxian-ctrl wants to merge 597 commits intonus-cs2113-AY2021S1:masterfrom
Conversation
kiathwe97
left a comment
kiathwe97
left a comment
There was a problem hiding this comment.
In general, insufficient sample input/outputs, but the rest of the DG is rather well done! Really neat overall presentation and appropriate levels of design details.
|
|
||
| public class Main { | ||
|
|
||
| private TextUi ui = new TextUi(); |
There was a problem hiding this comment.
In your class diagram, there is no need to put ui as an attribute and also use a line to denote composition. Just the line would do
| @@ -0,0 +1,241 @@ | |||
| package seedu.smarthomebot.logic.parser; | |||
There was a problem hiding this comment.
DG mentions class diagram. Should be Sequence Diagram instead?
ychong032
left a comment
ychong032
left a comment
There was a problem hiding this comment.
An area to consider for improvement is the consistency of the visuals used. For example, some images have colours while others do not. Some images are also a bit repetitive. On the other hand, the developer guide is ultimately very comprehensive and detailed.
|
|
||
| The class diagram of `Data Component` is shown below: | ||
|
|
||
|  <br> |
There was a problem hiding this comment.
Should the arrowheads of the arrows pointing from the various appliance classes (Light, AirConditioner etc) to the Appliance class be unfilled (i.e. filled with white)?
|
|
||
| The sequence diagram for `OnCommand` is shown below: | ||
|
|
||
|  <br> |
There was a problem hiding this comment.
Should the sequence diagrams be separated into different images for easier viewing? Would it be better to use colours?
|
|
||
| The class diagram of `Data Component` is shown below: | ||
|
|
||
|  <br> |
There was a problem hiding this comment.
In the composition relationships, is the multiplicity for the "whole" object necessary?
| The rest of the App consists of four components. | ||
| * `Ui`: The user interface where user can enter instructions and view output. | ||
| * `Logic` The command executor which consists of, | ||
| * `Paser`: Extract the keyword from user input |
| #### Sequence Diagram for `on` | ||
|
|
||
|  | ||
|
|
||
| #### Sequence Diagram for `off` | ||
|
|
||
|  | ||
|
|
||
| #### Sequence Diagram for `list` | ||
|
|
||
| <br><br> | ||
| When the user enters the `list` command, the | ||
| `prepareListCommand(arguments)` is called. It will check if the argument contains “appliance’ or ‘location”. | ||
| 1. If the arguments contains “location”, a new `ListCommand(LOCATION_TYPE, ““)` will be returned. | ||
|
|
||
| 2. If the arguments contains “appliance”, it will check if it contains a “l/” parameter. If it exist, it | ||
| means there is a filteredLocation, thus a new `ListCommand(APPLIANCE_TYPE, filteredLocation)` will be returned. | ||
| Else, a new `ListCommand(APPLIANCE_TYPE, ““)` will be returned. | ||
|
|
||
| 3. Any argument that does not contains “location” and “appliance” or contains “appliance” with a wrong | ||
| format will return a `InvalidCommand` class with their respective error messages. | ||
|
|
||
| #### Sequence Diagram for `commandword` | ||
|
|
||
|  | ||
|
|
||
| `commandword` refers to the following commands `help`, `usage`, `p_reset`, `exit`. | ||
| As these 4 commands does not require any additional parsing. The sequence diagram referred above will return their | ||
| respective CommandObject to execute the command. | ||
|
|
||
|
|
||
| #### Sequence Diagram for `default` | ||
|
|
||
|  |
There was a problem hiding this comment.
Are these sequence diagrams a bit repetitive?
|
|
|
|
| * Help: `help` | ||
| * Create a location: `create` | ||
| * Remove a location: `remove` | ||
| * Add an appliance: `add` | ||
| * Delete an appliance: `delete` | ||
| * Switch ON an appliance: `on` | ||
| * Switch OFF an appliance: `off` | ||
| * Listing appliance/location: `list` | ||
| * Displaying usage of appliance: `usage` | ||
| * Resetting usage of appliance: `p_reset` | ||
| * Exiting the application: `exit` |
There was a problem hiding this comment.
You may indicate the corresponding input format here for clearity
| #### Sequence Diagram for `on` | ||
|
|
||
|  |
There was a problem hiding this comment.
You may add a very short description for this part even though it is quite clear what this method does
| #### Sequence Diagram for `off` | ||
|
|
||
|  |
There was a problem hiding this comment.
You may add a very short description for this part even though it is quite clear what this method does
| # Developer Guide | ||
|
|
||
| ## Design & implementation | ||
| ## Table of Contents |
There was a problem hiding this comment.
Overall very clear DG for me to understand what your product about
|
Overall good DG |
|
|
Updated invalid type error message
Updated create command part in UG for better clarity
# Conflicts: # src/test/java/seedu/smarthomebot/logic/commands/RemoveCommandTest.java
Added Junit Test and Assertion
Update to UserGuide and Developer Guide
Updating image in UserGuide
# Conflicts: # docs/UserGuide.md
Update DG
Updated PPP
Updates for Printing Style
Minor adjustments for printing
PPP updated
Minor edit on the PPP and DG
Fix on format error
Minor amendment on personal PPP
PPP updates
Final DeveloperGuide.md update
Update of PPP for V2.1
9 participants
SmartHomeBot helps disabled working adults to consolidate all of the home appliance’s control into a centralised system. Users can also review and monitor electricity usage.